Add TUI table overrides for 5 high-traffic list commands#4732
Add TUI table overrides for 5 high-traffic list commands#4732simonfaltum wants to merge 20 commits intomainfrom
Conversation
Co-authored-by: Isaac
Co-authored-by: Isaac
|
Commit: 5158ea6
18 interesting tests: 9 SKIP, 7 KNOWN, 2 flaky
Top 20 slowest tests (at least 2 minutes):
|
shreyas-goenka
left a comment
There was a problem hiding this comment.
Note: This review was posted by Claude (AI assistant). Shreyas will do a separate, more thorough review pass.
Priority: LOW — Clean mechanical PR
This is a straightforward PR that adds TUI table overrides for 5 commands. No bugs or correctness issues found.
Minor Nits
- Unused
listReqvariables: Several overrides declarelistReq(e.g.,listReq := &catalog.ListCatalogsRequest{}) that is never used after flag binding. Consider using_or removing if unneeded. - Consistency: The pattern matches the existing override style in the codebase, which is good.
What looks good
- Clean, consistent pattern across all 5 overrides
- Correct use of
tableview.RegisterConfigfor each command - Proper column selection for each resource type
No blocking issues.
Treat pipeline search input literally when users type SQL LIKE wildcards, and add package-level override tests so nested SDK field access is exercised before runtime.
Previously, the TUI search callback overwrote req.Filter entirely with a name LIKE expression, discarding any filter the user passed via --filter. Now the name LIKE clause is combined with the existing filter using AND, so both constraints apply together.
Several list override functions declared a named request parameter (e.g. listReq) that was never used after flag binding. Replace these with blank identifiers to satisfy go vet and make intent clearer. The parameter is kept named in clusters, jobs, pipelines, and workspace overrides where it is actively used for flag binding or search closures.
Search input now triggers server-side filtering automatically after the user stops typing for 200ms, instead of waiting for Enter. This prevents redundant API calls on each keystroke while keeping the text input responsive. Enter still executes search immediately, bypassing the debounce. Uses Bubble Tea's tick-based message pattern with a sequence counter to discard stale debounce ticks when the user types additional characters before the delay expires.
Wrap the user-provided filter in parentheses before appending the AND clause. Without this, a filter like 'a OR b' combined with a name search would parse as 'a OR (b AND name LIKE ...)' instead of the intended '(a OR b) AND name LIKE ...'. Add a test case with an OR filter to verify correct parenthesization.
Previously, RenderIterator and RunPaginated only returned the error from tea.Program.Run(), ignoring any fetch error stored in the model. An API error mid-stream would display an error screen in the TUI but the command would still exit 0. Now both functions inspect the final model via the new Err() accessor and return the fetch error if set. Also documents the destructive MaxWidth truncation behavior on ColumnDef and renderContent.
Fix four issues in the paginated TUI: 1. Entering search mode now sets loading=true to prevent maybeFetch from starting new fetches against the shared iterator while in search mode. In-flight fetches are discarded via the generation check. 2. executeSearch sets loading=true (was false) to prevent overlapping fetch commands when a quick scroll triggers maybeFetch before the first search fetch returns. 3. Pressing esc to close search now restores savedRows, savedIter, and savedExhaust (same as clearing the query via enter with empty input). 4. RenderIterator now checks the final model for application-level errors via the new FinalModel interface, since tea.Program.Run() only returns framework errors.
…ui-overrides-2 # Conflicts: # libs/cmdio/render.go
Pipeline event messages can contain embedded newlines, carriage returns, and tabs that corrupt tab-delimited text output and TUI table rows. Add a `sanitize` template function to cmdio's renderFuncMap and use it in the text template. Also sanitize in the TUI Extract function. Increase MaxWidth from 60 to 200 so diagnostic payloads are not truncated destructively before the actionable part of the error. Co-authored-by: Isaac
Remove the PaginatedModel type alias (FinalModel interface suffices). Remove the duplicate TestPaginatedErrAccessor test that overlaps with TestPaginatedModelErr. Reduce the 5-line MaxWidth truncation comment to a single line. Co-authored-by: Isaac
shreyas-goenka
left a comment
There was a problem hiding this comment.
Stamping. I'm assuming the TUI UI is better.
|
|
||
| // sanitizeControlWhitespace replaces newlines and tabs with spaces to prevent | ||
| // corrupting tab-delimited text output. | ||
| func sanitizeControlWhitespace(s string) string { |
There was a problem hiding this comment.
This function is also defined in cmd/workspace/pipelines/overrides.go. Consolidate?
Suggested reviewersBased on git history of the changed files, these people are best suited to review:
Confidence: high Eligible reviewersBased on CODEOWNERS, these people or teams could also review: @andrewnester, @anton-107, @databricks/eng-apps-devex, @denik, @lennartkats-db, @shreyas-goenka Suggestions based on git history of 37 changed files (11 scored). See CODEOWNERS for path-specific ownership rules. |
Why
PR #4731 added curated TUI table overrides for 15 list commands. This follow-up covers 5 additional commands that are among the most frequently used in the CLI, but were missing curated columns.
Changes
Before: these 5 commands used either generic text templates (secrets, cluster-policies) or raw JSON output (lakeview, pipeline events) with no curated TUI table columns.
Now: all 5 register curated TableConfig overrides so they show useful columns in the interactive TUI. Commands that had no text template override (lakeview list, pipelines list-pipeline-events) also get template annotations for the non-interactive fallback.
This PR stacks on #4731. It only adds per-command overrides, no engine changes.
Post-review fixes
\n,\r,\t) in pipeline event messages to prevent table row corruptionPaginatedModeltype alias, useFinalModelinterface insteadTestPaginatedErrAccessortestTest plan
go build ./...make checkspassesmake lintfullpasses (0 issues)secrets list-scopes,lakeview list,pipelines list-pipeline-events